Add `Drop::pin_drop` for pinned drops #144537

frank-king · 2025-07-27T14:31:16Z

This PR is part of the pin_ergonomics experiment (the tracking issue is #130494). It allows implementing Drop with a pinned self receiver, which is required for safe pin-projection.

Implementations:

At least and at most one of drop and pin_drop should be implemented.
No direct call of drop or pin_drop. They should only be called by the drop glue.
pin_drop must and must only be used with types that support pin-projection (i.e. types with #[pin_v2]).
Allows writing fn drop(&pin mut self) and desugars to fn pin_drop(&pin mut self). (Will be in the next PRs)

frank-king · 2025-09-21T04:19:22Z

The CI failed because Drop::drop becomes a provided method. I'm afraid it might not be a good way to hack into librustdoc. Is there any other good way to fix it?

rustbot · 2025-11-07T14:18:20Z

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-11-07T14:18:22Z

⚠️ Warning ⚠️

Some commits in this PR modify submodules.

If this was not intentional, see I changed a submodule on accident in the rustc dev guide.

bors · 2025-11-10T22:55:40Z

☔ The latest upstream changes (presumably #148397) made this pull request unmergeable. Please resolve the merge conflicts.

petrochenkov · 2025-11-12T14:19:46Z

compiler/rustc_hir_analysis/messages.ftl


 hir_analysis_coercion_between_struct_single_note = expected a single field to be coerced, none found

+hir_analysis_conflict_impl_drop_and_pin_drop = conflict implementation of `Drop::drop` and `Drop::pin_drop`


Suggested change

hir_analysis_conflict_impl_drop_and_pin_drop = conflict implementation of `Drop::drop` and `Drop::pin_drop`

hir_analysis_conflict_impl_drop_and_pin_drop = conflicting implementations of `Drop::drop` and `Drop::pin_drop`

petrochenkov · 2025-11-12T14:27:10Z

compiler/rustc_hir_typeck/src/callee.rs

+    if tcx.is_lang_item(trait_id, LangItem::Drop)
+        // Allow calling `Drop::pin_drop` in `Drop::drop`
+        && let Some(parent) = tcx.opt_parent(body_id)
+        && !tcx.is_lang_item(parent, LangItem::Drop)


Suggested change

&& !tcx.is_lang_item(parent, LangItem::Drop)

&& !tcx.is_lang_item(tcx.parent(body_id), LangItem::Drop)

Body id should have a parent, but if it somehow doesn't, we certainly shouldn't skip the code below.

Also, calling the destructor explicitly should probably be unsafe?

petrochenkov · 2025-11-12T14:44:58Z

compiler/rustc_hir_analysis/src/check/always_applicable.rs

+    let mut pin_drop_span = None;
+    for &impl_item_id in item_impl.items {
+        let impl_item = tcx.hir_impl_item(impl_item_id);
+        if let hir::ImplItemKind::Fn(fn_sig, _) = impl_item.kind {


Some HIR is accessed on a good path here, ideally queries should be used and HIR accesses should be avoided for better incremental compilation (or only performed when errors are reported).

Do you mean using queries like asspciated_items or I should add a new query like this?

rustc_queries! { query drop_impl_fn_items(impl_id: DefId) -> DropImplFnItems { ... } } pub struct DropImplFnItems { pub drop: Option<DefId>, pub pin_drop: Option<DefId>, }

Or let the query return spans of drop and pin_drop directly?

Using existing queries like associated_items should be enough.

petrochenkov · 2025-11-12T14:47:01Z

tests/ui/drop/missing-drop-method.stderr

  --> $DIR/missing-drop-method.rs:2:1
   |
 LL | impl Drop for DropNoMethod {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `drop` in implementation


Maybe we can report the old error if tcx.features().pin_ergonomics() is not enabled?

petrochenkov · 2025-11-12T14:48:04Z

The src/doc/book submodule update should be removed.

petrochenkov · 2025-11-12T14:49:46Z

Let's run benchmarks first.
I'm mostly concerned about a new method being added to Drop, which increases the amount of codegen and sizes of vtables.

@bors try @rust-timer queue

rust-timer · 2025-11-12T14:49:49Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

rust-bors · 2025-11-12T14:49:50Z

🔒 Merge conflict

This pull request and the base branch diverged in a way that cannot
be automatically merged. Please rebase on top of the latest base
branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository,
you can resolve the conflict following these steps:

git checkout feature/pin-drop (switch to your branch)
git fetch upstream HEAD (retrieve the latest base branch)
git rebase upstream/HEAD -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self feature/pin-drop --force-with-lease (update this PR)

You may also read
Git Rebasing to Resolve Conflicts by Drew Blessing
for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub.
It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is
handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

petrochenkov · 2025-11-12T14:50:23Z

Needs a rebase for perf run.
@rustbot author

rustbot · 2025-11-12T14:50:31Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

frank-king · 2025-11-12T23:06:15Z

The src/doc/book submodule update should be removed.

One test would fail without that change, see
#144537 (comment).

petrochenkov · 2025-11-13T10:00:08Z

The src/doc/book submodule update should be removed.

One test would fail without that change, see #144537 (comment).

Then the change needs to be merged into the book's master first (right now I'm not sure where the new submodule is pointing to).

frank-king force-pushed the feature/pin-drop branch from 362f769 to 1497185 Compare July 27, 2025 14:31